Overview

This PR introduces two critical security modifiers to the TestArbitrage contract, enhancing access control and input validation for trade execution. These modifiers provide a robust defense layer against unauthorized access and invalid trade parameters.

Changes

1. Access Control Modifier - onlyAuthorized

Added a new modifier that restricts trade execution to authorized traders or the contract owner.

Implementation:

• Checks if msg.sender is in the authorizedTraders mapping OR is the contract owner
• Reverts with descriptive error message if unauthorized
• Prevents unauthorized users from executing trades

Security Benefits:

• Ensures only trusted addresses can execute arbitrage trades
• Adds an additional security layer on top of existing ownership controls
• Prevents potential exploitation from unauthorized external callers

2. Trade Parameter Validation Modifier - validTradeParams

Added a comprehensive validation modifier that checks all trade parameters before execution.

Validations Implemented:

• Router Validation:

  • Exactly 2 routers must be provided
  • Neither router address can be zero address

• Token Validation:

  • Exactly 2 tokens must be provided
  • Tokens must be different from each other
  • Neither token address can be zero address

• Fee Validation:

  • Fee must be greater than 0 and not exceed 10000 (100%)

• Profit Threshold Validation:

  • Must be within bounds: MIN_PROFIT_BPS <= minProfitBps <= MAX_BPS

• Slippage Validation:

  • Maximum slippage cannot exceed MAX_SLIPPAGE_BPS

• Deadline Validation:

  • Trade deadline must be in the future (>= current block timestamp)

Security Benefits:

• Prevents execution with malformed or malicious parameters
• Guards against common attack vectors (zero addresses, expired deadlines)
• Ensures economic parameters are within safe bounds
• Provides clear, descriptive error messages for debugging

3. Documentation

Both modifiers include comprehensive NatSpec documentation:

• Clear descriptions of functionality
• Parameter documentation
• Developer notes on security implications

4. Code Formatting

Applied forge fmt to ensure consistent code styling across the contract.

Files Changed

• src/onchain/TestArbitrage.sol - Added modifiers section with 36 new lines

Testing Recommendations

When reviewing/merging this PR, consider adding tests for:

1. Unauthorized access attempts (should revert)
2. Each validation condition in validTradeParams (boundary testing)
3. Valid trade execution with proper authorization and parameters
4. Edge cases (zero addresses, expired deadlines, invalid ranges)

Security Impact

Risk Level: Low (adds security, doesn't remove any)

These modifiers strengthen the contract's security posture by:

• Preventing unauthorized trade execution
• Validating all inputs before state changes
• Providing fail-fast behavior with clear error messages

Breaking Changes

None. These modifiers are additions and don't modify existing functionality until applied to functions.

Next Steps

These modifiers should be applied to relevant trade execution functions in subsequent commits to enforce the validation and access control rules.